fix(eap): convert deprecated aggregation nested in aggregation-filter formulas#8093
Merged
Conversation
… formulas AggregationComparisonFilterWrapper.accept only visited the filter's own top-level `aggregation` field — it never traversed into the columns of a `comparison_filter.formula`. As a result, a deprecated `aggregation` nested inside an aggregation-filter formula was never upgraded to `conditional_aggregation`, and `_column_to_expression` later rejected it with "Column is not one of: aggregate, attribute key, formula, or conditional_formula". This broke failure_rate()-style HAVING clauses (count(...) / count(...) > x) sent to EndpointTraceItemTable, e.g. api.organization-events and api.dashboards.tablewidget. The same formula in the SELECT columns worked, because ColumnWrapper already recurses into formula.left/right. Recurse into the comparison filter's formula columns so the conversion applies there too, matching ColumnWrapper's behavior. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
MeredithAnya
force-pushed
the
meredith/6-22-26-filter
branch
from
25d7209 to
f16b6b0
Compare
phacops
approved these changes
Jun 23, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
failure_rate()-style HAVING clauses sent toEndpointTraceItemTablefail with:This happens for any request whose
aggregation_filter.comparison_filter.formulacontains a column using the deprecatedaggregationfield (rather thanconditional_aggregationwith no filter - see sentry-protos comment)Root cause
The request pipeline runs
AggregationToConditionalAggregationVisitorto upgrade every deprecatedaggregationinto the newerconditional_aggregationbefore the query is built, so the rest of the pipeline only deals with one kind.AggregationComparisonFilterWrapper.acceptonly visited the filter's own top-levelaggregationfield — it never traversed into the columns ofcomparison_filter.formula. So a deprecatedaggregationnested inside the filter's formula was never converted, and_column_to_expressionrejected it.The same formula in the SELECT
columnsworks, becauseColumnWrapper.acceptalready recurses intoformula.left/formula.right. Only the aggregation-filter copy broke.Fix
Recurse into the comparison filter's formula columns, matching
ColumnWrapper's behavior.Test
Added
test_convert_aggregation_to_conditional_aggregation_in_comparison_filter_formula, which mirrors the failing query (a comparison filter comparing a formula with a nested deprecatedaggregation). It fails without the fix and passes with it.🤖 Generated with Claude Code